-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor InserterMenu & VisualEditorInserter; allow recent/frequent reusable blocks to be inserted #4224
Conversation
If insertable blocks are largely state-derived, I agree it's sensible that they could be implemented as selectors. One potential issue I see with the current implementation is with memoization via |
Note from #4139 (review): We should remove the reusable block from |
82d95a1
to
641f983
Compare
✨ This is ready to be properly reviewed now. |
I looked into this and My gut says that it's OK to leave this as is since registering a block type is unlikely to happen after the editor is instantiated—no? |
641f983
to
fb2669f
Compare
ef4dc56
to
e73eb4a
Compare
Changes the main inserter and the quick inserter to both use common selectors which determine which items that should appear for insertion. This means that common logic (e.g. is this block enabled?, has this block already been used?) can be shared between the two inserters. At the same time, also allow Reusable Blocks to appear in the MRU and MFU inserter lists.
When a reusable block is deleted, remove that block from recentInserts and frequentInserts.
Use the underlying block type's icon as the icon that displays in an inserter for a reusable block.
e73eb4a
to
d884f71
Compare
Going to split this work into smaller PRs that are less intimidating to review. |
See #4497 |
🕺 What
Currently,
InserterMenu
,VisualEditorInserter
, andblocksAutocompleter
all directly callgetBlockTypes()
andgetReusableBlocks()
to determine what blocks should be shown as something that can be inserted.This means that we duplicate some logic in three different places:
useOnce
set, whether it's been already used or notisPrivate
setThis approach is pretty error-prone: it's very easy to change the behaviour of one inserter and forget about the other three inserters.
To start addressing this, I've changed
InserterMenu
andVisualEditorInserter
to source their blocks from three new selectors:getInserterItems
- returns all items that are insertable, filters out private and disabled blocks, correctly setsisDisabled
getRecentInserterItems
- returns recently inserted items (e.g. for use in the Recent tab), filters out private and disabled blocks, correctly setsisDisabled
getFrequentInserterItems
- returns frequently inserted items (e.g. for use inVisualEditorInserter
), filters out private and disabled blocks, correctly setsisDisabled
All three selectors select from both dynamic (reusable) blocks and static blocks. That means that we fix #3793 and fix #4221.
Having one place to assemble how a reusable block appears in an inserter lets us also easily fix #4303.
📅 Future work
The next step is to make
blocksAutocompleter
pull fromgetInserterItems
. I've not done this yet as doing so would rely on being able to access editor state from theblocks
module—something that we haven't figured out yet.✅ How to test